Closes #137 - Add xportr_process() to apply all and write
#200
Conversation
|
I wrote a very basic version of this function to kick off discussion. It's really just a pipeline calling all the other functions in order. I think if someone really wants a function like this, they could easily write it themselves, and it adds another layer of complexity that feels like it has little payoff. Are there other aspects that could be added to this function to make it more substantial and add more value? |
|
I think the wrapper could be very handy for users who are not used to writing R functions. Also, I typically want everything in xportr when building the xpts so one call to rule them all is nice!! |
|
2 more options to consider?
|
I like the reasoning for just calling it |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #200 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 14 +1
Lines 627 636 +9
=========================================
+ Hits 627 636 +9 ☔ View full report in Codecov by Sentry. |
EeethB
force-pushed
the
137_apply_all_write
branch
from
846642e
to
8b46126
Compare
EeethB
force-pushed
the
137_apply_all_write
branch
from
8b46126
to
9afeeb5
Compare
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
Co-authored-by: André Veríssimo <[email protected]>
|
See #199 (comment) I won't touch this PR, but it seems that merging that branch here solves all the CI errors 😄 |
| suppressMessages( | ||
| xportr_metadata(adsl, var_spec, domain = "adsl") %>% xportr_type() | ||
| xportr_metadata(adsl, var_spec, domain = "adsl", verbose = "none") %>% |
There was a problem hiding this comment.
is the suppressMEssages still need if this local_message_sink is used?
There was a problem hiding this comment.
I just had a couple of thoughts on presentation of xportr() in readme and the use of local_message_sink and suppressMessages. Other than that...LGTM!!
@averissimo any other concerns?
Thank you for your Pull Request!
We have developed a Pull Request template to aid you and our reviewers. Completing the below tasks helps to ensure our reviewers can maximize their time on your code as well as making sure the xportr codebase remains robust and consistent.
The scope of
{xportr}{xportr}'s scope is to enable R users to write out submission compliantxptfiles that can be delivered to a Health Authority or to downstream validation software programs. We see labels, lengths, types, ordering and formats from a dataset specification object (SDTM and ADaM) as being our primary focus. We also see messaging and warnings to users around applying information from the specification file as a primary focus. Please make sure your Pull Request meets this scope of {xportr}. If your Pull Request moves beyond this scope, please get in touch with the{xportr}team on slack or create an issue to discuss.Please check off each task box as an acknowledgment that you completed the task. This checklist is part of the Github Action workflows and the Pull Request will not be merged into the
mainbranch until you have checked off each task.Changes Description
Task List
stylerpackage and functions to style files accordingly.devtools::document()so all.Rdfiles in themanfolder and theNAMESPACEfile in the project root are updated appropriatelypkgdown::build_site()and check that all affected examples are displayed correctly and that all new/updated functions occur on the "Reference" page.vbump.yamlCI.